CEXT-6079: add unit tests for lib-app runtime actions#412
CEXT-6079: add unit tests for lib-app runtime actions#412vinayrao2000 wants to merge 3 commits intoadobe:mainfrom
Conversation
|
github-actions
Bot
added
without-changeset
There was a problem hiding this comment.
I only reviewed a couple of files for now. Probably the comments I added there also apply to the others.
Let's keep the tests focused to testing very specific things instead of having macro tests that assert multiple different things. Also make sure that you only "assert" those things that the test title describes, otherwise it can generate confusion if that test starts failing because of a change that seems unrelated 👍🏻
| expect(initializeMock).toHaveBeenCalledWith({ schema: configSchema }); | ||
| expect(byScopeIdMock).toHaveBeenCalledWith("store-1"); | ||
| expect(getConfigurationMock).toHaveBeenCalledWith( | ||
| { scopeId: "store-1" }, | ||
| { encryptionKey: "encryption-key" }, | ||
| ); |
There was a problem hiding this comment.
These lines seem to assert implementation details that this test should not care about. I'd remove them
| }), | ||
| ); | ||
|
|
||
| expect(initializeMock).toHaveBeenCalledWith({ schema: configSchema }); |
There was a problem hiding this comment.
Idem on this initialize check. Don't think this test needs to care whether we call initialize or not.
| { scopeId: "store-1" }, | ||
| { encryptionKey: undefined }, |
There was a problem hiding this comment.
Let's assert only the parameters we want to actually check (in this case the first one). For others we can just expect.any(Object) or something like that. This way if those parameters are different in the future, these tests won't break.
| expect(result).toEqual({ | ||
| type: "success", | ||
| statusCode: 200, | ||
| headers: { | ||
| "Cache-Control": "no-store", | ||
| Deprecation: "Wed, 15 Apr 2026 00:00:00 GMT", | ||
| }, | ||
| body: { | ||
| scopeId: "store-1", | ||
| config: [ | ||
| { name: "apiKey", value: "*****", origin: "scope" }, | ||
| { name: "mode", value: "live", origin: "scope" }, | ||
| ], | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Similarly, I would not assert the result in this test. I would only do it in a test that is intended to do that.
| expect(result).toEqual({ | ||
| type: "success", | ||
| statusCode: 200, | ||
| body: { | ||
| schema: configSchema, | ||
| values: { | ||
| scopeId: "store-1", | ||
| config: [ | ||
| { name: "apiKey", value: "*****", origin: "scope" }, | ||
| { name: "mode", value: "live", origin: "scope" }, | ||
| ], | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
You should probably only assert the result in a specific test for the 200 result.
| statusCode: 200, | ||
| headers: { | ||
| "Cache-Control": "no-store", | ||
| Deprecation: "Wed, 15 Apr 2026 00:00:00 GMT", |
There was a problem hiding this comment.
I would add a specific test only for this header, to ensure future updates don't delete it.
| }); | ||
| }); | ||
|
|
||
| test("forwards partial updates and null unsets with PATCH /", async () => { |
There was a problem hiding this comment.
This test it's a bit confusing to me. I would refactor it to two separate tests that assert the following:
- PUT doesn't accept
nullvalues - PATCH does accept
nullvalues.
3 participants
CEXT-6079: add unit tests for lib-app runtime actions
Description
Added a focused unit test suite for REST runtime actions in
@adobe/aio-commerce-lib-appto cover handler behavior and regression scenarios.Added test files
packages/aio-commerce-lib-app/test/fixtures/actions.tspackages/aio-commerce-lib-app/test/unit/actions/app-config.test.tspackages/aio-commerce-lib-app/test/unit/actions/config.test.tspackages/aio-commerce-lib-app/test/unit/actions/scope-tree.test.tspackages/aio-commerce-lib-app/test/unit/actions/installation.test.tsCoverage scope
appConfigRuntimeActionconfigRuntimeActionscopeTreeRuntimeActioninstallationRuntimeActionThe tests are unit-level with mocked dependencies and no external service calls.
Related Issue
CEXT-6079
Motivation and Context
lib-appREST runtime action behavior needed direct automated coverage for request/response paths and error handling.This change improves confidence in action logic and catches regressions without requiring external integration setup.
How Has This Been Tested?
Executed from repo root:
pnpm testpnpm typecheckBoth completed successfully in the local workspace.
Types of changes
Checklist: